-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bugfix: cancel and speedup insufficient funds #1252
Bugfix: cancel and speedup insufficient funds #1252
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
const { accounts } = this.props; | ||
const checksummedFrom = safeToChecksumAddress(tx.transaction.from); | ||
const balance = accounts[checksummedFrom].balance; | ||
return hexToBN(balance).lt( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might want to wrap this in a try & catch and return true(?) on the catch.
The reason is that there are several values involved (gas, tx value, rate, gas price) and those come from different sources, if one of those requests fail, the whole thing will fail too, and I'd prefer in that case to fail later rather than prohibiting everyone
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
totally
The disabled cta works good 👍 However, when I sanity checked speeding up txn's when I have plenty of funds, I get a possible unhandled promise rejection seen here = http://recordit.co/e1x3fXNeTI |
fixed #1252 (comment) . on gaba |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix looks good on both OS's, QA passed 👍
* alert message * snaps * fix * bump gaba * try catch
Description
Instead of showing an alert I blocked the button to do a cancel / speed up as you can see here
https://recordit.co/uPCGfIzRqW
Once address book pr we can use the same error component here
Checklist
Issue
Resolves #1208